Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verify 1104 pipeline #1154

Closed
wants to merge 4 commits into from
Closed

Verify 1104 pipeline #1154

wants to merge 4 commits into from

Conversation

ivanleomk
Copy link
Collaborator

@ivanleomk ivanleomk commented Nov 8, 2024

Important

Enhance token usage tracking by adding detailed token metrics in initialize_usage and update_total_usage.

  • Enhancements:
    • Add CompletionTokensDetails and PromptTokensDetails to CompletionUsage in initialize_usage() in retry.py.
    • Update update_total_usage() in utils.py to handle audio_tokens and reasoning_tokens in completion_tokens_details, and audio_tokens and cached_tokens in prompt_tokens_details.

This description was created by Ellipsis for 821afdf. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Reviewed everything up to 821afdf in 19 seconds

More details
  • Looked at 43 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 4 drafted comments based on config settings.
1. instructor/retry.py:71
  • Draft comment:
    Consider using Union from the typing module instead of | for type unions to maintain compatibility with Python versions below 3.10.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code in instructor/retry.py and instructor/utils.py has a few issues related to type checking and potential runtime errors. The main issue is the use of Any in type hints, which can lead to less strict type checking and potential runtime errors. Additionally, the use of | for type unions is not compatible with Python versions below 3.10, which could lead to compatibility issues if the code is run in such environments.
2. instructor/utils.py:140
  • Draft comment:
    Consider using Union from the typing module instead of | for type unions to maintain compatibility with Python versions below 3.10. This is applicable in other parts of the file as well.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code in instructor/utils.py has a similar issue with type unions using |. This should be addressed for compatibility with Python versions below 3.10.
3. instructor/retry.py:17
  • Draft comment:
    If CompletionUsage is part of the library code, ensure that the documentation and tests are updated to reflect the changes made to include completion_tokens_details and prompt_token_details. This applies to other instances where CompletionUsage is used.
  • Reason this comment was not posted:
    Comment did not seem useful.
4. instructor/utils.py:140
  • Draft comment:
    Ensure that the changes to update_total_usage to include completion_tokens_details and prompt_token_details are reflected in the documentation and tests. This applies to other instances where update_total_usage is used.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_OFoA7ZY4f5QFmibH


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@github-actions github-actions bot added enhancement New feature or request size:M This PR changes 30-99 lines, ignoring generated files. labels Nov 8, 2024
Copy link

Deploying instructor-py with  Cloudflare Pages  Cloudflare Pages

Latest commit: 821afdf
Status: ✅  Deploy successful!
Preview URL: https://9a85d0a2.instructor-py.pages.dev
Branch Preview URL: https://verify-1104-pipeline.instructor-py.pages.dev

View logs

@ivanleomk ivanleomk closed this Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants